Code Review Bench PR #30339 - feat: get plan bulk with cache by ketkarameya · Pull Request #10 · gitarcode/dify

ketkarameya · 2026-02-10T21:26:22Z

Code Review Bench PR langgenius#30339 - feat: get plan bulk with cache

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#10

feat: get plan bulk with cache

Summary by Gitar

New caching method: Added BillingService.get_plan_bulk_with_cache() with Redis-backed caching (10-minute TTL) to reduce unnecessary billing API calls in batch scenarios
Optimized batch operations: Uses Redis mget() for cache reads and pipeline() for cache writes to minimize round-trips
Graceful degradation: Falls back to existing get_plan_bulk() API for cache misses, handles all Redis failures transparently without raising exceptions
Enhanced error handling: Improved get_plan_bulk() to skip invalid tenants rather than returning None, making it more robust as a fallback mechanism
Comprehensive test coverage: Added 9 integration tests with real Redis (testcontainers) plus 1 unit test covering all cache hit/miss scenarios and failure modes (4.4:1 test-to-code ratio)

_{This will update automatically on new commits.}

gitar-bot-staging · 2026-02-10T21:38:28Z

api/services/billing_service.py

+                        logger.exception(
+                            "get_plan_bulk: failed to validate subscription plan for tenant(%s)", tenant_id
+                        )
+                        results[tenant_id] = None


⚠️ Bug: get_plan_bulk sets None instead of skipping invalid tenants

On line 292, when validation fails for a tenant's plan data, the code sets results[tenant_id] = None. This contradicts the intended behavior (skip invalid tenants) and has two downstream consequences:

Test failure: The unit test test_get_plan_bulk_with_invalid_tenant_plan_skipped asserts "tenant-invalid" not in result and len(result) == 2, but the code adds the invalid tenant with value None, so len(result) will be 3 and the assertion will fail.

Type violation: get_plan_bulk is typed as returning dict[str, SubscriptionPlan], but it returns None values. This propagates into get_plan_bulk_with_cache which will add None to tenant_plans, cache "null" in Redis (via json.dumps(None)), and on the next cache read, validate_python(None) will fail, causing a perpetual miss-refetch-cache cycle for that tenant.

The fix is to use continue instead of assigning None.

_{Was this helpful? React with 👍 / 👎}

Suggested change

results[tenant_id] = None

except Exception:

logger.exception(

"get_plan_bulk: failed to validate subscription plan for tenant(%s)", tenant_id

)

continue

Apply suggested fix

gitar-bot-staging · 2026-02-10T21:38:31Z

Note

Gitar has paused automatic reviews. Comment "Gitar review" to trigger a review manually.

Code Review ⚠️ Changes requested 0 resolved / 2 findings

The get_plan_bulk error handler at line 292 assigns None instead of skipping invalid tenants, violating the return type, failing the corresponding unit test, and causing a cache pollution cycle when used by get_plan_bulk_with_cache. Replace results[tenant_id] = None with continue.

⚠️

Bug: get_plan_bulk sets None instead of skipping invalid tenants

📄 api/services/billing_service.py:292

On line 292, when validation fails for a tenant's plan data, the code sets results[tenant_id] = None. This contradicts the intended behavior (skip invalid tenants) and has two downstream consequences:

Test failure: The unit test test_get_plan_bulk_with_invalid_tenant_plan_skipped asserts "tenant-invalid" not in result and len(result) == 2, but the code adds the invalid tenant with value None, so len(result) will be 3 and the assertion will fail.
Type violation: get_plan_bulk is typed as returning dict[str, SubscriptionPlan], but it returns None values. This propagates into get_plan_bulk_with_cache which will add None to tenant_plans, cache "null" in Redis (via json.dumps(None)), and on the next cache read, validate_python(None) will fail, causing a perpetual miss-refetch-cache cycle for that tenant.

The fix is to use continue instead of assigning None.

Suggested fix

                    except Exception:
                        logger.exception(
                            "get_plan_bulk: failed to validate subscription plan for tenant(%s)", tenant_id
                        )
                        continue

💡 Edge Case: None values from get_plan_bulk cached and returned as plans

📄 api/services/billing_service.py:365

Since get_plan_bulk currently sets results[tenant_id] = None for validation failures (line 292), get_plan_bulk_with_cache will blindly iterate over these None entries at line 365-367, adding them to both tenant_plans (returned to the caller) and plans_to_cache. Then json.dumps(None) produces "null", which gets cached in Redis. On the next cache read, json.loads("null") returns None, and validate_python(None) will fail, treating it as a cache miss — causing a repeated cache-miss → fetch → cache-"null" cycle.

This is a direct consequence of the line 292 bug. Once line 292 is fixed to continue instead of assigning None, this issue is also resolved. However, as a defensive measure, you could also add a None check in the caching loop:

for tenant_id, subscription_plan in bulk_plans.items():
    if subscription_plan is not None:
        tenant_plans[tenant_id] = subscription_plan
        plans_to_cache[tenant_id] = subscription_plan

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

gitar-bot-staging

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

kayagokalp · 2026-02-24T22:39:20Z

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 1/1 = 100.0%)

ID	Severity	Title	Found?
GT-DIFY-10-001	Critical	Invalid tenants added to results with None instead of being	Yes

Extra Gitar Findings

File	Line	Valid Bug?	Notes
api/services/billing_service.py	365	Yes	None values from get_plan_bulk cached and returned

Summary

Recall: 100.0% (1/1)
Precision: 100.0% (2/2)
Precision counts ground truth matches + valid extra findings as true positives

hj24 and others added 4 commits January 25, 2026 12:05

feat: get plan bulk with cache

cc91077

[autofix.ci] apply automated fixes

a49d980

Update billing_service.py

47e4b2d

update pr

70fe873

gitar-bot-staging bot reviewed Feb 10, 2026

View reviewed changes

gitar-bot-staging bot requested changes Feb 10, 2026

View reviewed changes

kayagokalp closed this Feb 25, 2026

kayagokalp deleted the corrupted_pr_30339_20260125_4193 branch February 25, 2026 02:06

kayagokalp restored the corrupted_pr_30339_20260125_4193 branch February 25, 2026 02:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Code Review Bench PR #30339 - feat: get plan bulk with cache#10

Code Review Bench PR #30339 - feat: get plan bulk with cache#10
ketkarameya wants to merge 4 commits intobase_pr_30339_20260125_4193from
corrupted_pr_30339_20260125_4193

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading

Uh oh!

gitar-bot-staging bot left a comment

Uh oh!

kayagokalp commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Conversation

ketkarameya commented Feb 10, 2026 • edited by gitar-bot-staging bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Bench PR langgenius#30339 - feat: get plan bulk with cache

Summary by Gitar

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitar-bot-staging bot left a comment

Choose a reason for hiding this comment

Uh oh!

kayagokalp commented Feb 24, 2026

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 1/1 = 100.0%)

Extra Gitar Findings

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading